Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

v2.x: Improve x86-64 timer performance #3748

Merged
merged 2 commits into from
Jul 6, 2017
Merged

v2.x: Improve x86-64 timer performance #3748

merged 2 commits into from
Jul 6, 2017

Conversation

matcabral
Copy link
Contributor

porting PR #2926 to v2.x

@matcabral matcabral requested a review from hppritcha June 23, 2017 23:21
@matcabral matcabral added this to the v2.1.2 milestone Jun 23, 2017
@bosilca
Copy link
Member

bosilca commented Jun 26, 2017

I found an interesting reading on this issue (http://akaros.cs.berkeley.edu/lxr/akaros/kern/arch/x86/rdtsc_test.c). We might want to have a reference on it somewhere.

jsquyres
jsquyres requested changes Jun 26, 2017
View reviewed changes
Copy link
Member

@jsquyres jsquyres left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matcabral Can you please put the "cherry picked from" notations in the commits? You should be able to use the -x option to git cherry-pick to have git automatically put those in there for you.

This helps us track what has gone over from master to the release branches.

@matcabral
opal_progress: use usec native timer only when a native cycle counter…
1aead8b 
… isn't

available
Signed-off-by: Carlos Bederián <bc@famaf.unc.edu.ar>
(cherry picked from commit 4009ba6)
@matcabral
x86_64 timers: use lfence instead of cpuid for serialization
37b30fa 
Signed-off-by: Carlos Bederián <bc@famaf.unc.edu.ar>
(cherry picked from commit  ccea3de)
@jsquyres jsquyres changed the title Improve x86-64 timer performance v2.x: Improve x86-64 timer performance Jun 27, 2017
@matcabral matcabral mentioned this pull request Jun 27, 2017
Merged
@jsquyres jsquyres requested a review from hjelmn June 27, 2017 14:46
jsquyres
jsquyres approved these changes Jun 27, 2017
View reviewed changes
Copy link
Member

@jsquyres jsquyres left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These commits are actually authored by @zzzoom and (effectively) cherry-picked over here (#3768 shows the attribution properly for v2.0.x). But this is probably good enough.

@hjelmn Can you sanity check these?

@bosilca
Copy link
Member

bosilca commented Jun 27, 2017

The changes in opal/runtime/opal_progress.c are good. I have added a comment regarding the lack of documentation on why lfence+rdtsc is a better approach.

@jsquyres
Copy link
Member

@bosilca Thanks for the sanity check. 👍

@hppritcha
Copy link
Member

@hjelmn please double check this PR.

@hppritcha hppritcha merged commit 4846629 into open-mpi:v2.x Jul 6, 2017
@matcabral matcabral mentioned this pull request Aug 9, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsquyres jsquyres jsquyres approved these changes

@hppritcha hppritcha hppritcha approved these changes

@hjelmn hjelmn Awaiting requested review from hjelmn

Assignees
No one assigned
Labels
bug performance RM approved
Projects
None yet
Milestone
v2.1.2
Development

Successfully merging this pull request may close these issues.
4 participants
@matcabral @bosilca @jsquyres @hppritcha